Clarify as_uninit_mut may point to uninitialized memory#158804
Clarify as_uninit_mut may point to uninitialized memory#158804yilin0518 wants to merge 1 commit into
as_uninit_mut may point to uninitialized memory#158804Conversation
rustbot
added
S-waiting-on-review
|
r? @LawnGnome rustbot has assigned @LawnGnome. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| /// When calling this method, you have to ensure that *either* the pointer is null *or* | ||
| /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). | ||
| /// Note that because the created reference is to `MaybeUninit<T>`, the | ||
| /// source pointer can point to uninitialized memory. |
There was a problem hiding this comment.
This formulation seems backwards to me, and this information also seems already to be included in "In contrast to [as_mut], this does not require that the value has to be initialized." which is a few lines above this.
There was a problem hiding this comment.
The purpose of the change I made is let the safety documentation of this API consistent with other APIs(*mut T::as_uninit_ref, *const T::as_uninit_ref and std::ptr::NonNull::as_uninit_mut). All these APIs has this pattern in their safety documentation. Although "In contrast to [as_mut], this does not require that the value has to be initialized." has implied the uninitiazlition, it seems that explicitly emphasize this in #Safety is better. And thank you for your review and feedback!
There was a problem hiding this comment.
I see. Also, you're welcome to the review and thanks for trying to improve the docs and their consistency in particular.
I'm not sure I agree this needs to be repeated in the SAFETY section, as it is not a safety condition that will have to be dispatched in a safety comment using these methods. The MaybeUninit type already makes it very difficult to accidentally mishandle MaybeUninit<T> as T.
Anyway, assuming something like this is included in the SAFETY section, I would prefer a reformulation. From "Note that because the created reference is to MaybeUninit<T>, the source pointer can point to uninitialized memory." to perhaps one of:
- Remember that the returned reference may refer to an uninitialized
T. - Note: returned reference may refer to an uninitialized
T.
Or if the point is instead about the argument: selfmay point to uninitialized memory.
What do you think?
There was a problem hiding this comment.
From "Note that because the created reference is to MaybeUninit<T>, the source pointer can point to uninitialized memory." to perhaps one of:
- Remember that the returned reference may refer to an uninitialized
T.- Note: returned reference may refer to an uninitialized
T.
Or if the point is instead about the argument:selfmay point to uninitialized memory.What do you think?
Well I think reformulation is optional, but adding the uninitialization hint in SAFETY section is better than in other place, because SAFETY section needs to remind developers that what they should care about when using this API.
But I think the first discription is great if needing reformulation. It is more concise. And the other point we should care is that the documentation of these APIs need modification if reformulation is accepted. I can finfish the work and apply in this PR.
4 participants
Tracking issue: #75402
This PR clarifies the safety documentation for
*mut T::as_uninit_mut.Unlike
as_mut, this method creates a reference toMaybeUninit<T>, so the source pointer may point to uninitialized memory. The pointer still must either be null or satisfy the pointer-to-reference conversion requirements.After adding the supplementary information, the safety documentation of
*mut T::as_uninit_mutis consistent with*mut T::as_uninit_ref,*const T::as_uninit_refandstd::ptr::NonNull::as_uninit_mut.This is a documentation-only change.
Tests were not added because this only updates API docs.